Skip to content

Conversation

@martinflorian-da
Copy link
Contributor

…tionTest`

Fixes DACH-NY/cn-test-failures#6839

[ci]

Signed-off-by: Martin Florian <martin.florian@digitalasset.com>
[ci]

Signed-off-by: Martin Florian <martin.florian@digitalasset.com>
@martinflorian-da martinflorian-da marked this pull request as ready for review January 21, 2026 14:40
Copy link
Contributor

@moritzkiefer-da moritzkiefer-da left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

def replaceStringsInJson(viewValue: Json) = {
val current = viewValue.spaces2SortKeys
val amuletRulesId = sv1ScanBackend.getAmuletRules().contractId.contractId
val amuletRulesId = eventually()(sv1ScanBackend.getAmuletRules().contractId.contractId)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the eventually for? If it's for handling the command failure from it not existing (not sure why it would not exist?) don't you need an eventuallySucceeds?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I meant to comment on this line before marking ready, sorry...

what is the eventually for?

Just a cheap extra safety against flakes... this is the thing that failed previously. I don't care here if scan has a random hiccup.

don't you need an eventuallySucceeds

Yes, thanks for catching that!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original issue was that this call failed due to rate limiting. Why is only this call being rate limited? Can we cache the amulet rules to reduce the amount of calls?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just a dumb call from out of the test itself... I wouldn't overthink this tbh; we're not testing rate limiting here and the thing that is causing too much load on scan is not part of our app logic; just the test logic.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean we could implement caching inside of the test but the gain should just be a minor test duration improvement... doesn't seem worth it to me.

martinflorian-da and others added 3 commits January 21, 2026 15:51
…ation/tests/TokenStandardCliTestDataTimeBasedIntegrationTest.scala

Co-authored-by: moritzkiefer-da <45630097+moritzkiefer-da@users.noreply.github.com>
Signed-off-by: Martin Florian <martin.florian@digitalasset.com>
[static]

Signed-off-by: Martin Florian <martin.florian@digitalasset.com>
@martinflorian-da martinflorian-da enabled auto-merge (squash) January 21, 2026 14:58
[force]

Signed-off-by: Martin Florian <martin.florian@digitalasset.com>
@martinflorian-da martinflorian-da merged commit ae4c264 into main Jan 21, 2026
7 of 44 checks passed
@martinflorian-da martinflorian-da deleted the martinflorian-da/cntf-6839-dont-rate-limit-simtime-test branch January 21, 2026 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants